feat: add OG image for compare pages#2277
feat: add OG image for compare pages#2277Adebesin-Cell wants to merge 28 commits intonpmx-dev:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a server-rendered OG-image Vue SFC at Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8bf4f3f9-b99e-4b4a-82b3-eb8cf42d26e8
📒 Files selected for processing (2)
app/components/OgImage/Compare.vueapp/pages/compare.vue
Fetches real weekly downloads and latest version for each package, renders a horizontal bar chart with gradient bars sized relative to the highest-downloaded package. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Show descriptive text when no packages are selected, matching the Default template's badge style. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Pass i18n-translated empty description from the page as a prop since OG image components don't have access to $t. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/components/OgImage/Compare.vue (2)
41-69: Top-level await reads computed value during setup.The code accesses
displayPackages.valueat the top level of<script setup>, which evaluates during component setup. While this works in Nuxt OG image server-rendering contexts where props are available immediately, this pattern can be fragile if the component is ever used in a different context.Consider using
onMountedor Nuxt's data-fetching composables if issues arise, but for a server-only OG image component, the current approach should work correctly.
138-138: Consider potential duplicate key collision.Using
:key="pkg.name"assumes package names are unique. If the same package name appears multiple times in the input (e.g.,?packages=react,react), this would cause a key collision warning and potentially incorrect rendering.For an OG image component, this edge case is unlikely to matter, but you could use the index as a fallback:
💡 Optional fix
- <div v-for="pkg in stats" :key="pkg.name" class="flex flex-col gap-1"> + <div v-for="(pkg, index) in stats" :key="`${pkg.name}-${index}`" class="flex flex-col gap-1">
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89a5a390-89bf-49d9-b45a-2cf8521a47c3
📒 Files selected for processing (3)
app/components/OgImage/Compare.vueapp/pages/compare.vuetest/unit/a11y-component-coverage.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/pages/compare.vue
|
Great work on the new compare OG image! Perhaps we could give the metrics (weekly downloads) a bit more visual prominence? I personally think the download numbers are the most important info here — the progress bars already do a great job showing the relative scale, but the absolute numbers are what people scan for first. |
… image Bump download metrics from text-xl/gray to text-2xl/font-medium/lighter color so absolute numbers are easier to scan at a glance.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/OgImage/Compare.vue (1)
22-29: Normalisestring[]input the same as comma-separated input.Line 28 passes array input through as-is, so whitespace-only or empty entries can leak into fetch calls. Normalising both paths keeps behaviour consistent.
♻️ Suggested refactor
const displayPackages = computed(() => { const raw = props.packages - const list = - typeof raw === 'string' - ? raw - .split(',') - .map(p => p.trim()) - .filter(Boolean) - : raw + const list = (typeof raw === 'string' ? raw.split(',') : raw) + .map(p => p.trim()) + .filter(Boolean) return list.slice(0, 4) })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f198a43d-d3b0-4229-9a86-9da70bcb40d8
📒 Files selected for processing (1)
app/components/OgImage/Compare.vue
|
Amazing 🤩 Is there a limit for how many packages can be compared at once or could this be abused to generate OG images that overflow the image dimensions? |
There is currently a limit of 4 on the compare page, but we'd like to either increase the limit or remove it entirely eventually. We discussed about this on discord, and @Adebesin-Cell has a plan for various scenarios to avoid overflows with different presentations (to be implemented when it becomes necessary). |
Thanks for the quick answer and ping. This looks perfect to me then 👍 |
- Add 2.5s timeout to npm API requests to prevent OG image render stalls - Increase download numbers to text-3xl font-bold white for better visual prominence on the compare OG card Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Full tier (1-4): name + downloads + version badge + bar - Compact tier (5-6): name + downloads + thinner bar, no version - Grid tier (7-12): side-by-side grid layout (4×2 or 5×2) - Summary tier (13+): top 3 names + "+N more" fallback - Extended accent colors to 12 for larger comparisons - Skip version fetch for non-full tiers (faster rendering) - Extract magic numbers into named constants Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
| ) | ||
| stats.value = results | ||
| } catch { | ||
| stats.value = displayPackages.value.map((name, index) => ({ |
There was a problem hiding this comment.
I recommend you sort the stats by downloads, to show bars and lists from highest to smallest
- Sort packages by downloads descending for readability - Use consistent package box icon matching Package/Default OG images - Match icon size (w-16 h-16 p-3.5) to other OG components Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Show version badges on full/compact tiers (after package name) - Show inline version text on grid tier - Fetch versions for all tiers (not just full) - Sort packages by downloads descending for readability - Add ./npmx branding in bottom-right corner - Fix icon outline by using explicit linear-gradient Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
graphieros
left a comment
There was a problem hiding this comment.
I fixed the confllct on the compare page.
Can you please fix the failing test in FacetSelector.spec.ts ? You just need to move the function out of scope as per the error message.
There is also a minor css class thing to fix as well.
Thank you:)
- Lighten version and /wk text in grid layout (#525252/#737373 → #a3a3a3) - Move findCategoryActionButton out of describe block (consistent-function-scoping) - Fix formatting in Compare.vue and compare.vue Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Hey! Thanks for fixing the conflicts. I totally forgot I had this PR open. I fixed the tests now + made the texts more visible |
|
@Adebesin-Cell
After this, it will be LGTM |
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Ha, thanks! |
There was a problem hiding this comment.
Code lgtm, thanks 🤍
Can't check sth like https://npmx-fww36c0b2-npmx.vercel.app/compare?packages=zowe-revolutionizes-mainframe-devops-with-seamless-integration-streamlining-workflows-enabling-collaboration-automating-processes-enhancing-efficiency-and-accelerating-software-delivery-for-enhanced-business-value,sdfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff but not related to changes
Names over 40 chars are truncated with ellipsis to prevent layout overflow with extremely long package names in all layout tiers. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Use numeric gap values (Satori-compatible) instead of CSS string gaps - Use flex-wrap with 220px items for grid layout (4 columns) - Add maxWidth on name spans for reliable text truncation - Remove version from grid tier to prevent layout misalignment - Redesign summary tier: smaller "13 packages" text to not compete with title - Improve /wk text visibility (#d4d4d4) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Better social preview for compare pages.